Skip to content

Conversation

@rbtr
Copy link
Collaborator

@rbtr rbtr commented Mar 12, 2025

don't restart HNS when setting SDNRemoteArpMacAddress (or at all, it's fragile).
reverts behavior changed in #3343 (timeout and retry if HNS is not responding) and #2993 (restart HNS after the regkey is set unconditionally)

Copilot AI review requested due to automatic review settings March 12, 2025 15:34
@rbtr rbtr requested a review from a team as a code owner March 12, 2025 15:34
@rbtr rbtr requested a review from paulyufan2 March 12, 2025 15:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes the HNS service restart functionality when setting SDNRemoteArpMacAddress to improve reliability by avoiding fragile service restarts.

  • Removed unused imports (svc and svc/mgr)
  • Removed the restartHNS function and its invocation in SetSdnRemoteArpMacAddress

@rbtr rbtr requested a review from chandanAggarwal March 12, 2025 15:35
@rbtr rbtr self-assigned this Mar 12, 2025
@rbtr rbtr added cns Related to CNS. fix Fixes something. windows labels Mar 12, 2025
@rbtr rbtr force-pushed the fix/softer-hns-restart branch from 64335c6 to 7cd7a04 Compare March 12, 2025 22:27
@rbtr rbtr changed the title fix: remove HNS restart fix: don't restart HNS if the ARP regkey is not changed Mar 12, 2025
@rbtr
Copy link
Collaborator Author

rbtr commented Mar 12, 2025

/azp run Azure Container Networking PR

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rbtr rbtr requested a review from Copilot March 12, 2025 22:29
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR modifies the behavior when setting the SDNRemoteArpMacAddress registry key so that the HNS service is not restarted if the registry value is already set.

  • The setSDNRemoteARPRegKey function now returns a boolean flag indicating if the key was changed.
  • The SetSdnRemoteArpMacAddress function has been updated to check this flag and conditionally skip restarting HNS.

@rbtr rbtr enabled auto-merge March 12, 2025 22:34
@rbtr rbtr force-pushed the fix/softer-hns-restart branch from 7cd7a04 to 46a5d4a Compare March 13, 2025 00:02
@rbtr rbtr requested a review from a team as a code owner March 13, 2025 00:02
@rbtr
Copy link
Collaborator Author

rbtr commented Mar 13, 2025

/azp run Azure Container Networking PR

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rbtr rbtr added this pull request to the merge queue Mar 13, 2025
Merged via the queue into master with commit 2b6f593 Mar 14, 2025
14 checks passed
@rbtr rbtr deleted the fix/softer-hns-restart branch March 14, 2025 13:18
sivakami-projects pushed a commit that referenced this pull request Oct 23, 2025
fix: skip HNS restart if ARP regkey unchanged

Signed-off-by: Evan Baker <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cns Related to CNS. fix Fixes something. windows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants